fix: per-issue conclusion concurrency in issue-duplication-detector#2318
fix: per-issue conclusion concurrency in issue-duplication-detector#2318
Conversation
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
AWF creates audit files (squid.conf, docker-compose.redacted.yml, policy-manifest.json) as root with 0o600 permissions. When gh-aw's post-job secret scanner runs as the runner user, it gets EACCES trying to stat/scan these files, causing job failures. Since audit files already have secrets redacted, change permissions from 0o700/0o600 to 0o755/0o644 so they're readable without needing the chmod a+rX cleanup step to have run first. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Security Review: Audit Artifact Permission RelaxationFinding (Low Severity): The audit directory and its files are now created world-readable from the start ( For the default case this is safe: The concern is with custom Edge-case names (e.g. Suggested action: No change needed for the common path. Consider adding a code comment at line 2342 noting that the No issues found in the GitHub Actions workflow changes (concurrency group scoping) or the post-processing script additions.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Scopes the issue-duplication-detector workflow’s conclusion job concurrency group to the triggering issue (or run_id fallback) to prevent GitHub Actions concurrency-group eviction/cancellation when issues are opened in batches. Also adds a post-compile postprocessor transform + tests to keep the lock workflow corrected after future gh aw compile runs. This PR additionally changes AWF audit artifact permissions in writeConfigs().
Changes:
- Update
issue-duplication-detector.lock.ymlconclusion job concurrency group to be per-issue (issue.number || run_id). - Add an idempotent postprocess transform in
postprocess-smoke-workflows.tsto re-apply the concurrency fix after compilation. - Add unit tests validating the regex/transform behavior and idempotency.
Show a summary per file
| File | Description |
|---|---|
src/docker-manager.ts |
Changes audit directory + audit artifact file permissions to be world-readable. |
scripts/ci/postprocess-smoke-workflows.ts |
Adds a targeted transform to scope issue-duplication-detector conclusion concurrency per issue. |
scripts/ci/postprocess-smoke-workflows.test.ts |
Adds tests for the new postprocess regex/transform (and idempotency guard). |
.github/workflows/issue-duplication-detector.lock.yml |
Applies the per-issue concurrency group fix in the compiled lock workflow. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
src/docker-manager.ts:2356
- The new 0o644 modes for audit artifacts assume the files are always safe to disclose to all local users. However,
redactDockerComposeSecrets()only redacts environment variable values based on key name patterns and does not guarantee that other compose fields (or unexpectedly named env vars) cannot contain sensitive values.
Suggestion: keep audit artifact files at 0o600 by default (and rely on cleanup-time chmod when needed for artifact upload), or strengthen the redaction guarantees before making them world-readable.
fs.writeFileSync(path.join(auditDir, 'squid.conf'), squidConfig, { mode: 0o644 });
// Save redacted docker-compose.yml (strip env vars that may contain secrets)
const redactedCompose = redactDockerComposeSecrets(dockerCompose);
fs.writeFileSync(
path.join(auditDir, 'docker-compose.redacted.yml'),
yaml.dump(redactedCompose, { lineWidth: -1 }),
{ mode: 0o644 }
);
- Files reviewed: 4/4 changed files
- Comments generated: 3
| // World-readable so gh-aw post-job scanners can access audit artifacts | ||
| // (files are already secret-redacted, so 0o755 is safe) | ||
| fs.mkdirSync(auditDir, { recursive: true, mode: 0o755 }); | ||
| } |
| // World-readable so gh-aw post-job scanners can access audit artifacts | ||
| // (files are already secret-redacted, so 0o755 is safe) | ||
| fs.mkdirSync(auditDir, { recursive: true, mode: 0o755 }); |
| // The regex itself still matches the suffix of the group name, but the | ||
| // sentinel guard in the script prevents double-application. |
|
@copilot address the review feedback |
Addressed all three review points in 9576d83:
|
|
Smoke Test Results ✅ ✅ GitHub MCP: Retrieved last 2 merged PRs Status: PASS
|
|
Smoke Test: Copilot BYOK (Offline Mode)
Running in BYOK offline mode ( PR by
|
🔥 Smoke Test Results
PR: fix: per-issue conclusion concurrency in issue-duplication-detector Overall: PASS
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
|
PR titles: Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
The Issue Duplication Detector had a ~40% cancellation rate when issues were opened in batches. All agent jobs completed successfully — only the final
conclusionjob was being cancelled.Root cause: The
conclusionjob used a single shared concurrency group across all workflow runs. GitHub Actions enforces a hard limit of 1 running + 1 pending job per group; with 7 runs completing simultaneously, each new arrival evicted the current pending job, cascading cancellations through the batch.Changes
issue-duplication-detector.lock.yml— Scope the conclusion job's concurrency group to the triggering issue number:github.run_idas fallback handlesworkflow_dispatchtriggers where no issue number exists.postprocess-smoke-workflows.ts— Add an idempotent transformation that re-applies this fix after any futuregh aw compilerecompilation. Uses flexible[ ]+whitespace matching to be resilient to YAML indentation changes.postprocess-smoke-workflows.test.ts— 5 new tests covering: pattern matching, transformation output, idempotency via sentinel, preservation ofcancel-in-progress: false, and group name prefix integrity.